Skip to content

Conversation

Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Sep 7, 2024

Related: #293 (hey there!)
This pull request was authored on behalf of @RiversideValley

Before the change?

  • Dependency: .NET Core 3.1 Runtime

After the change?

  • Dependency: .NET 8.0 Runtime

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes


Hello! Me and @0x5bfa are really interested in this project as it is used in our project @FluentHub
We'd like to propose a few changes, such as Native AOT (exciting! 🚀 but for another PR) and .NET 8 target runtime support. This PR updates the library/test runs to .NET 8. Not quite complete yet though.

Copy link

github-actions bot commented Sep 7, 2024

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@Lamparter

This comment was marked as resolved.

@mousemaid
Copy link

mousemaid commented Sep 7, 2024

The issue was fixed in e445b1c 💚

To view the logs, click here 😄
   ___  ___    _   ___ _    ___ 
  / _ \| _ \  /_\ / __| |  | __|
 | (_) |   / / _ \ (__| |__| _| 
  \___/|_|_\/_/ \_\___|____|___|
                                
"Connected to Oracle server in tenancy AD-2, South UK"
winget install Git.Git
winget install Microsoft.DotNet.SDK.8
[Riverside].Respire
"Found: Octokit.GraphQL.UnitTests"
"Found: Octokit.GraphQL.IntegrationTests"
"Other projects found, but are .NET Libraries. Will not deploy, but will still provide build results."
[Riverside].ApplicationHostServices
"Build started at 10:12..."
"Build completed at 10:13 and took 1 minutes"
[Riverside].ApplicationHostGateway
"Compiling log"
"Log returned to debug gateway"

Octokit.GraphQL.Core.UnitTests.ConnectionTests.Run_Specifies_Cancellation_Token
📋 Source:ConnectionTests.cs line 60
🕒 Duration:3 ms

System.Threading.Tasks.TaskCanceledException: A task was canceled.
System.Threading.Tasks.TaskCanceledException: A task was canceled.

HttpClient.HandleFailure(Exception e, Boolean telemetryStarted, HttpResponseMessage response, CancellationTokenSource cts, CancellationToken cancellationToken, CancellationTokenSource pendingRequestsCts)
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
Connection.Run(String query, CancellationToken cancellationToken) line 211
ConnectionTests.Run_Specifies_Cancellation_Token() line 73
HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)

@Lamparter Lamparter changed the title Feature: Upgrade .NET from Core 3.1 to 8.0 Feature: Upgrade .NET from Core 3.1 to 8.0 (and Native AOT!) Sep 7, 2024
@Lamparter

This comment was marked as resolved.

@kfcampbell
Copy link
Member

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

@Lamparter
Copy link
Contributor Author

Lamparter commented Sep 28, 2024

This path no longer exists because of the upgrade, causing an error with the CI
image

Is this part of ${{ env.OG_PACK_PROJ_PATH }}?

@0x5bfa
Copy link
Contributor

0x5bfa commented Nov 8, 2024

@Lamparter I fixed the CI. Could you:

  • Tell me if this is already published AoT and works well?
  • Squash my these 17 commits? (GH Desktop > History > Select them > right click to Squash but you have to rebase first Branch > Rebase > Select origin/main)

@0x5bfa
Copy link
Contributor

0x5bfa commented Nov 8, 2024

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

@kfcampbell Are you wanting to bump the version up here?

Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
…sions.1.12.1.nupkg to src/Octokit.GraphQL.Core.UnitTests/Assets/AgileObjects.ReadableExpressions.1.12.1.nupkg

Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
@Lamparter
Copy link
Contributor Author

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

[redacted] Are you wanting to bump the version up here?

I already bumped it. They were talking about marking this pr as a breaking change because I'd updated it.

@martincostello
Copy link
Contributor

Any reason this is still in draft? Builds in CI are completely broken due to the dependency on the .NET Core 3.1 SDK. I could just bump the SDK in my PR, but it would make more sense to just wait on this if it's going to get finished/merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

5 participants